Multiple top-level Controllers per FastCS app (#351)#360
Multiple top-level Controllers per FastCS app (#351)#360
Conversation
Introduce a stable per-controller identifier set once by the launcher between __init__ and initialise(). Reading id before it is set raises a RuntimeError, and setting twice raises. __repr__ surfaces the id once set, and create_api_and_tasks now seeds the root ControllerAPI path with [id] so sub-APIs become [id, sub]. Backwards compatible: when id is unset (existing single-controller launcher path), the API path remains empty and behaviour is unchanged. Part of #353. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pure utility that derives an EPICS PV prefix from a controller path: the first segment (controller id) is used verbatim, while later segments are converted snake_case -> PascalCase. EPICS adopts this in #354 to replace the existing root-prefix-plus-pascalled-path approach for multi-controller IOCs. D2 module of #353. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reject controller ids that aren't safe in a REST URL path: empty or containing characters outside the loosest URL-safe set ([A-Za-z0-9_-]+). The error message includes the offending id so startup failures are unambiguous. Hookup into RestTransport.connect follows in the multi-controller routing slice. D3-REST module of #353. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Every transport's connect() now takes list[ControllerAPI] uniformly. The existing single-controller transports (EPICS CA, EPICS PVA, GraphQL, Tango, REST) accept a list-of-one via a shared _expect_single helper and behave as before. FastCS.serve passes [self.controller_api]. True multi-controller support per transport will be wired in subsequent slices. This is a pure refactor: existing tests are updated to the new list-of-one call shape, no behaviour changes for any transport. Part of #353. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
RestServer now accepts list[ControllerAPI] and adds attribute and
command routes for each. RestTransport hooks validate_rest_id into
connect() so illegal ids fail fast with a clear startup error. Existing
path-based routing already prefixes routes with controller_api.path[0],
so once Controller.set_id seeds the API path, REST URLs become
GET /{id}/{sub}/{attr} for free.
Two new tests in tests/test_multi_controller.py cover routing two
distinct ids in one process and rejecting an id with an illegal
character.
Part of #353.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`launch()` now accepts either a single Controller class or a list of classes; the generated `fastcs.yaml` schema replaces the top-level `controller:` key with a dict of `controllers:` keyed by id. Each value carries a `type:` discriminator (defaults to the class `__name__`, overridable via `type_name: ClassVar[str]`) and an optional `controller:` options block. Single-class registration may omit `type:` via a default. Duplicate ids are rejected at YAML load time by ruamel's safe loader. Wiring through `FastCS` for >1 controller lands in the next slice; for now multi-entry configs validate cleanly but the run command exits with a clear LaunchError pointing at the deferred work. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #360 +/- ##
==========================================
- Coverage 91.24% 91.18% -0.07%
==========================================
Files 70 72 +2
Lines 2604 2858 +254
==========================================
+ Hits 2376 2606 +230
- Misses 228 252 +24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| num_ramp_controllers: 4 | ||
| controllers: | ||
| TEMPERATURE: | ||
| controller: |
There was a problem hiding this comment.
@coretl I'd like your opinion on this - I think this is the right shape but it does not match with what transports do.
the schema is
controllers:
id_str_as_key:
type: class of controller (may be omitted if there is only one class)
controller:
init parameters for the controller classalso - it looks a little odd to me for the simple case of single controller (especially with no init args), like the above.
See discussion with Claude below.
The controller: key sits as a sibling to type: — it's the namespace that holds whatever Pydantic options model the Controller's init declared. So a
config entry has two halves:
- type: — FastCS metadata (which class to instantiate)
- controller: — the author's own options model, plugged in intact as the type they declared
The real tension I see: transports use discriminator-as-key (graphql: { host: ... }), but controllers use type: + controller:. There's an inconsistency in
the YAML's mental model — one discriminated-union shape would be nicer. The discriminator-as-key version of controllers would be:
controllers:
TEMPERATURE:
TempController: # discriminator IS the key
ip_settings: { ip: localhost, port: 25565 }
num_ramp_controllers: 4
Cleaner, parallel to transports, removes the "redundant layer" feel. Cost: harder to add per-id FastCS metadata later without re-introducing a wrapper, and
harder to default type: away in the single-class case (today _build_entry_model makes type literal-with-default so controllers: { TEMPERATURE: { controller:
{...} } } works without naming the class).
My recommendation: keep the current shape. The visual redundancy is a one-line learning curve; the architectural property — the author's options class lives
in its own namespace, untouched by FastCS metadata — pays off the first time we add an entry-level field. But if you'd rather optimise for "the YAML reads
cleanly first time" and live with the constraints, discriminator-as-key is a defensible call and worth doing now before slice #357 lands and lots of
docs/tests reference the shape.
FastCS.__init__ now accepts Controller | Sequence[Controller]; serve() loops initialise/post_initialise/connect/disconnect over every controller, builds list[ControllerAPI], and hands the full list to each transport.connect(). IPython context exposes parallel dicts (controllers, controller_apis) keyed by controller id (falling back to class name when no id is set), and the startup log line lists controller ids. fastcs.controller_api singular accessor is replaced with the controller_apis list. The temporary >1-controller LaunchError stub in launch._launch.run is removed; multi-entry configs are wired through FastCS directly. Single Controller direct construction continues to work via the union arg, so docs snippets are unchanged. A new end-to-end test in tests/test_multi_controller.py drives FastCS.serve with two id-tagged controllers and a RestTransport, asserts all four lifecycle hooks fire on each, and verifies /<id>/<attr> routing plus combined OpenAPI through TestClient. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EpicsCATransport now hosts every configured controller in a single softioc, with each controller's id used verbatim as its PV prefix. EpicsCAIOC takes list[ControllerAPI] and loops the existing record/PVI/command builders per controller, deriving each prefix from pv_prefix_from_path(api.path) (the D2 utility introduced in #353). EpicsCATransport.connect drops _expect_single in favour of true multi-controller; validate_ca_id runs at connect time and rejects ids with illegal characters as well as setups whose longest derivable PV prefix already exceeds the 60-character EPICS limit. EpicsIOCOptions and its pv_prefix field are deleted. EpicsCAOptions and EpicsPVAOptions empty placeholders preserve epicsca: / epicspva: as fastcs.yaml discriminator keys (Pydantic union resolution is positional, so a unique field name per transport is still load-bearing). EpicsGUI no longer takes a separate prefix argument; PVs derive from the controller path. PVA temporarily continues via _expect_single but adopts pv_prefix_from_path so it gets the same id-based prefix and no longer needs EpicsIOCOptions; full PVA multi-root work lands in #355. tests/test_multi_controller.py grows a CA two-controllers-no-clash scenario and a CA id-validation fail-fast case. tests/example_softioc, tests/example_p4p_ioc, tests/benchmarking/controller, tests/conftest, test_initial_value, test_p4p, test_softioc, test_gui, test_pva_gui and the AssertableControllerAPI fixture all migrate to id-based naming (controllers set their id, transports take no prefix). Demo controller.yaml and both regenerated schema.json files reflect the new EpicsCAOptions / EpicsPVAOptions schemas and removal of pv_prefix. The 13 docs/snippets are exercised by tests/test_docs_snippets.py via runpy, so they migrate in this commit too to keep the suite green at every commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
multiple-transports.md and launch-framework.md still showed EpicsIOCOptions(pv_prefix=...) in their Python and YAML examples. Replace those with the id-based shape: controllers set their id (or inherit it from the YAML controllers: dict key), and EpicsCATransport / EpicsPVATransport take no prefix argument. The prose follows the API that landed in the previous commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
EpicsPVATransport now hosts every configured controller in one p4p server, with each controller's id used verbatim as its PV prefix. P4PIOC takes list[ControllerAPI] and builds one StaticProvider per controller via the existing parse_attributes helper, so each controller gets an independent :PVI root with no super-parent (per the PRD). EpicsPVATransport.connect drops _expect_single in favour of true multi-controller; validate_pva_id runs at connect time and rejects ids with illegal characters as well as setups whose longest derivable PV prefix already exceeds the 60-character EPICS limit. validate_pva_id mirrors validate_ca_id and lives in transports/epics/ pva/util.py to keep id validation a per-transport concern. To share the 60-char constant without a cross-transport import, EPICS_MAX_NAME_LENGTH moves up from ca/util.py to epics/util.py; ca/util.py re-imports it so existing ca.util consumers (ca/ioc.py, test_softioc) are unaffected. tests/test_multi_controller.py grows a PVA two-controllers-distinct-PVI scenario (asserts each StaticProvider exposes its own root) and a PVA id-validation fail-fast case. test_pva_util.py mirrors test_ca_util.py's validator coverage. test_p4p.py::test_pvi_grouping shortens its UUID id to 8 hex chars so the deepest derived prefix (<id>:AdditionalChild:ChildChild) no longer trips the new 60-char check. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sphinx is configured with `nitpicky = True` and `--fail-on-warning`, so single-backticks in the new P4PIOC docstring (`StaticProvider`, `:PVI`) were treated as :any: cross-references and failed to resolve. Switch to double-backticks so they're inline literals instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the GraphQL transport into the multi-controller foundation: - New `validate_graphql_id` enforces GraphQL `Name` syntax (the most restrictive of FastCS's transports — drives the lowest-common-denominator id-naming guidance for users mixing transports). - `GraphQLServer` now accepts `list[ControllerAPI]` and assembles a single combined schema with one top-level Query (and Mutation, where applicable) field per controller id. Sub-API type names are path-joined to keep two controllers' identically-named sub-controllers from clashing in the schema. - `GraphQLTransport.connect` validates ids fail-fast at startup. - `tests/test_multi_controller.py` gains a two-controller combined-schema scenario and a per-transport id-validation case. - The single-controller transport test is updated to namespace its queries under a controller id, matching the new contract; a latent bug in its `nest_mutation` helper (recursing through `nest_query`) is fixed in passing. - `docs/how-to/multiple-transports.md` adds a charset table and notes GraphQL as the lowest common denominator for cross-transport ids.
D4 of #351 lands as a single transport-level module. Both EPICS transports now invoke `emit_gui_files(controller_apis, options, builder)` once with the full controller list, replacing the per-controller loop that wrote everything to the same file. The module produces: - One screen/docs file per controller at `output_dir/{id}.{ext}`, preserving the order in which controllers were declared in `fastcs.yaml`. - An index file at the root of `output_dir` -- emitted even for a single controller, so the file layout is stable as the controller count changes. The GUI index uses pvi's `DLSFormatter` directly (rather than the convenience `format_index` wrapper) so that `DeviceRef.name` can be coerced to satisfy pvi's `PascalStr` constraint when controller ids legitimately start with a digit (e.g. UUID-flavoured test prefixes). The docs side mirrors the GUI shape with a minimal markdown emitter -- just enough to lift `EpicsDocs.create_docs` off its prior no-op stub. Knock-on schema/option changes: - `EpicsGUIOptions.output_path` (single file) becomes `output_dir` (directory); ditto `EpicsDocsOptions.path` -> `output_dir`. The per-controller filename is derived from the controller id. - The bundled demo, the 13 docs snippets, `tests/example_softioc.py`, the multi-transport how-to and the `launch-framework` how-to all migrate to the new field name. Both `schema.json` files are regenerated. - `EpicsGUI` loses its `create_gui()` file-writing entry point in favour of a smaller `build_device(title) -> Device` helper that the emission module composes per controller. - `tests/data/config.yaml` drops its `gui: {}` / `docs: {}` blocks -- they were schema-fixture noise that now leaks generated files into the repo CWD when the launcher tests exercise `connect()`. Tests: - New `tests/transports/epics/test_emission.py` (D4 unit tests): per-id files plus index for single and multi-controller cases, declaration order preservation, missing-output-dir creation, PVA builder propagation, and the digit-leading-id coercion. - `tests/transports/epics/ca/test_gui.py` gains a transport-level assertion that the index file is generated alongside per-controller files (per #358 acceptance criteria). - `tests/test_multi_controller.py` gains a CA scenario that drives `EpicsCATransport.connect` end-to-end and asserts both per-id and index files for GUI and docs land in their configured `output_dir`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8b7cf38 to
fc8e710
Compare
The #358 commit migrated the docs/snippets EpicsGUIOptions(...) calls from a 3-line to a 1-line form via ruff format, shifting every line below `gui_options = ...` up by 2 in static05/06/10/14/15.py. The tutorial's `:emphasize-lines:` references for those snippets in docs/tutorials/static-drivers.md were left pointing at the old positions, which made `sphinx -W` fail with two "line number spec is out of range" warnings on static05 and static10. Update each affected range so the highlighted lines correspond to the same code as before. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n guide
Bundled demo now hosts two TemperatureController instances (MAIN/AUX) on
distinct ports so the multi-controller feature is visible end-to-end. The
demo simulation grows a second TempController on port 25566 with its own
sink to drive the AUX controller; the main one stays on 25565.
Renames src/fastcs/demo/controller.yaml -> src/fastcs/demo/fastcs.yaml.
The launcher already takes the config path as a CLI argument, so nothing
hard-codes the new name. .vscode/launch.json updated; schema.json
unchanged (the dict-by-id form was already in place).
docs/how-to/launch-framework.md examples migrate to fastcs.yaml and gain
a "Hosting multiple controllers" section. New
docs/how-to/migrate-to-multi-controller.md covers the breaking-change
manual migration steps: file rename, controller: -> controllers:{id}
dict, EpicsIOCOptions.pv_prefix removal, type: discriminator with
single-class inference, GUI/docs output_dir rename.
Fixes #359
TangoTransport.connect now accepts list[ControllerAPI] without the
_expect_single guard. validate_tango_id (in transports/tango/util.py)
runs per-id at connect, rejecting characters outside [A-Za-z0-9_-] with
a clear startup error. New helpers tango_dev_class_name (id sanitised
into a valid Python class name: hyphens to underscores, leading-digit
prefix X) and tango_dev_name(id, dsr_instance) build the three-segment
"<id>/<dev_class>/<dsr_instance>" device name with the id as the
leading segment.
TangoDSR holds list[type] _devices, one Tango device class per
controller, and registers them via a single tango.server.run() call.
The DSR's server name is fixed to FASTCS_TANGO_SERVER_NAME ("FastCS")
so a multi-class server has a single identity independent of any one
controller's class. _collect_dev_attributes / _collect_dev_commands
now use the path relative to the device root (path[len(root):]) so
the leading id is the device name and not also folded into attribute
names — preserves single-controller attribute/command naming.
dev_name dropped from TangoDSROptions; the device name is fully
derived from the id and dsr_instance now, mirroring the
EpicsIOCOptions.pv_prefix removal in #354. register_dev grows an
optional server_name arg (defaults to dev_class for back-compat) and
a new register_controller_devs helper batch-registers every
controller under FASTCS_TANGO_SERVER_NAME.
Tests:
- tests/transports/tango/test_tango_util.py: validator + helper unit
coverage including hyphen replacement and leading-digit prefix.
- tests/test_multi_controller.py: two-controllers-distinct-devices
scenario + id-validation fail-fast on bad-char ids.
- tests/transports/tango/test_dsr.py: existing single-controller
coverage updated for path=["DEVICE"] root and the _devices list.
- tests/conftest.py register_device fixture and
tests/benchmarking/controller.py migrated off the dropped dev_name.
- Both schema.json files regenerated.
docs/how-to/multiple-transports.md gains a Tango row in the charset
table (the existing PVA row also gains the 60-char note in passing)
and shows the dsr_instance-only options. The PVA-row drive-by closes
half of #366.
Fixes #357
Each entry under `controllers:` now exposes the controller's options fields directly as siblings of `type:`, instead of a nested `controller:` block. Pydantic's discriminated union does the dispatch + per-class validation in one pass; `_instantiate_controllers` reduces to a one-liner that reconstructs the options-type from the validated entry. Options classes may be stdlib dataclasses, pydantic dataclasses, or BaseModels; unsupported shapes raise LaunchError. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Previously the discriminator defaulted to the registered class's discriminator when only one Controller class was registered, so single- class apps could omit `type:`. Mandate it everywhere: `type:` becomes required regardless of how many classes are registered. Self-describing YAML — external tooling can identify each entry's class without knowing how `launch()` was called — and one consistent shape across single- and multi-class apps. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pyright (CI lint job) flagged the previous approach of stashing ``fastcs_controller_class`` and ``fastcs_options_type`` directly on each dynamically-built Entry model class as ``reportAttributeAccessIssue``. Replace with a module-level ``_ENTRY_REGISTRY`` dict mapping Entry class → (Controller class, options-type), mirroring the ``Transport.subclasses`` pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The py:obj nitpick_ignore entry for fastcs.transports.epics.util.controller_pv_prefix was added when the docs build emitted a warning for that symbol. The symbol's referencing context was removed in c8adb33, so the ignore entry no longer suppresses anything and is dead config. Fixes #364 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "Hosting multiple controllers" subsection of `docs/how-to/launch-framework.md` introduced its yaml example by claiming the bundled demo (`python -m fastcs.demo run src/fastcs/demo/fastcs.yaml`) hosted two `DeviceController` instances on different ports. The actual demo hosts two `TemperatureController` instances with nested `ip_settings` and `num_ramp_controllers` options on the same IP but different ports — so the prose did not match the code it pointed at. Reframe the example as a generic `DeviceController` illustration (consistent with the rest of the file's placeholder), and add a separate one-liner pointing readers at `src/fastcs/demo/fastcs.yaml` for a real working example. The yaml block itself is unchanged. Fixes #370 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each docs/snippets/*.py wired up an EPICS CA transport but never called
controller.set_id(...). The snippets compiled and tests passed because
runpy only executes the module body — fastcs.run() lives behind the
if __name__ == "__main__": guard, so transport.connect never fires
and pv_prefix_from_path([]) never raises. A reader who copy-pastes a
snippet and tries to actually run it would hit ValueError: Cannot
derive a PV prefix from an empty path the moment FastCS starts.
Extract the controller into a local before constructing FastCS and
call controller.set_id("DEMO") so each snippet now matches the runtime
contract documented in docs/how-to/multiple-transports.md and
docs/how-to/launch-framework.md.
Fixes #363
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Remove the `if api.path:` guard around `validate_graphql_id(api.path[0])` in `GraphQLTransport.connect`. REST, EPICS CA, EPICS PVA and Tango all call `path[0]` unconditionally — a controller without a leading id is a configuration bug, not a path GraphQL should silently accept while the others reject. The other half of #366 — adding the 60-char PV-name limit to the PVA row of the charset table in `docs/how-to/multiple-transports.md` — was already applied as a drive-by in 207d68d (Tango multi-device per controller with id in device name). Fixes #366 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`_instantiate_controllers` previously inferred whether a Controller class takes an options arg by checking `options_type is None` — the build-time decision in `_build_entry_model` had to stay implicit and in sync with this check. Replace the registry's `(cls, options_type)` tuple with a `_RegisteredClass` dataclass that carries an explicit `expects_options: bool` alongside the class and options-type, so the runtime branch reads off the same source of truth as the build-time one. Fixes #361 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`tango_dev_class_name` collapses hyphens to underscores so that a controller id like `DEV-1` and `DEV_1` both sanitise to the same Python class name `DEV_1`. `tango.server.run` then registers two Tango device classes under that name and silently keeps only the second, so one of the controllers vanishes from the device server with no diagnostic. Catch this at the same boundary as the existing per-id Tango validation: `TangoTransport.connect` now records every sanitised class name it has seen and raises `ValueError` naming both colliding ids when a second id maps onto an existing one. `validate_tango_id` is deliberately left accepting hyphens — the underlying id is still legal, only the post-sanitisation class name clashes. Fixes #371 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
validate_ca_id and validate_pva_id were byte-for-byte identical except for their per-transport regex constant and the "EPICS CA id" / "EPICS PVA id" label in the illegal-characters error message. Move the structural skeleton into transports/epics/util.py as validate_epics_pv_id, parameterised on transport_label and id_re. Each transport's validator becomes a 3-line wrapper that supplies its own _CA_ID_RE / _PVA_ID_RE and label, so the regex constants stay co-located with their transport and can diverge later if either side needs to tighten or loosen its alphabet. Error messages and validation behaviour are identical; existing tests in tests/transports/epics/ca/test_ca_util.py and tests/transports/epics/pva/test_pva_util.py pass unchanged. Fixes #365 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous commit hoisted the shared EPICS id-validation skeleton into ``transports/epics/util.py`` but dropped the ``EPICS_MAX_NAME_LENGTH`` import from ``ca/util.py``, breaking consumers that imported it from the CA module's namespace (``ca/ioc.py`` and ``tests/transports/epics/ca/test_softioc.py``). Point those consumers at the canonical home in ``transports/epics/util`` instead of restoring the re-export. Fixes #365 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `src/fastcs/transports/epics/docs.py` module had already been reduced to a re-export shim for `emit_docs_files` and `EpicsDocsOptions`, with nothing in `src/`, `tests/`, or `docs/` importing from it. The CA and PVA transports both call `emit_docs_files` directly from `fastcs.transports.epics.emission` (which carries the real per-controller docs emission added in slice #358), and `EpicsDocsOptions` is re-exported from `.options` via `transports/epics/__init__.py`. Deleting the shim removes the last vestige of the old `EpicsDocs.create_docs` no-op stub. Fixes #367 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The IOC publishes PVs using the controller id verbatim (see pv_prefix_from_path in transports/epics/util.py), but the per-controller DeviceRef in the GUI index was upper-casing the id when writing the pv attribute. For id="alpha" the per-controller .bob referenced alpha:Foo (matching the IOC) while the index .bob referenced ALPHA, which the IOC never publishes. Drop the .upper() so the index agrees with the IOC. Fixes #368 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The EPICS CA id validator accepts `[A-Za-z0-9_-]+`, so ids like `"___"` and `"-"` pass validation and reach `_coerce_pascal_name` in the GUI emission path. That helper delegates to `pvi.device.enforce_pascal_case`, which strips non-Pascal characters and unconditionally indexes `s[0]` on the result. When every character is stripped the index raises `IndexError`, blowing up GUI emission at `connect()` time with an opaque traceback. Pre-strip the id with the same regex pvi uses (`NON_PASCAL_CHARS_RE`) and raise `ValueError` with a message that names the offending id when the strip yields the empty string. Choosing fail-fast over an `"X"` fallback keeps the failure traceable: a silent fallback would generate nonsense GUI names that the user would have to reverse-engineer back to the bad id. Fixes #369 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fixes #353
Fixes #354
Fixes #355
Fixes #356
Fixes #357
Fixes #358
Fixes #359
Foundation slice (#353) wires multiple top-level Controllers end-to-end through REST. The EPICS CA slice (#354) layers the first transport-side multi-root on top. The EPICS PVA slice (#355) layers the second transport-side multi-root on top. The GraphQL slice (#356) layers the third — one combined schema with id-keyed top-level Query fields. The Tango slice (#357) layers the fourth — one Tango Device Server hosts N devices, one per controller, with the id forming the leading device-name segment. The GUI/docs slice (#358) layers the fifth — per-id GUI/docs files plus an always-emitted index file. The demo+rename slice (#359) lands the user-facing surface — bundled demo hosts two controllers, the demo config is renamed
controller.yaml→fastcs.yaml, and a manual migration guide is added. All seven are on this branch, so each commit below names the slice it serves; commits within a slice are independent sub-modules and are green individually. Two further commits added during review tighten the YAML shape — flat per-id options with a requiredtype:discriminator — and are folded in here so the migration guide describes the final shape in one place.Fixes #353 — multi-controller foundation (REST tracer)
5bb366cc0d4a3296893487db6dd956de0b2e1985011bb683d4ff267eFixes #354 — EPICS CA multi-root softioc with id-based PV prefix
c8adb33bc1b95a29Fixes #355 — EPICS PVA multi-root with N PVI roots
439560505317473fFixes #356 — GraphQL combined schema with id-keyed top-level Query fields
e5217785Fixes #358 — GUI/docs emission: per-id files plus index file
fc8e710bf2c7cef9Fixes #359 — Demo, migration guide, and config file rename to fastcs.yaml
f6600bc9Fixes #357 — Tango multi-device with id in device name
207d68d8Refinements (in addition to the original PRD)
Tighten the multi-controller YAML shape introduced above: each entry under
controllers:now exposes the controller's options fields directly as siblings of a requiredtype:discriminator, instead of a nestedcontroller:block. Pydantic's discriminated union does the dispatch and per-class validation in one pass; consumers reading the published JSON schema can identify each entry's class without knowing howlaunch()was called.4de23e6b439f2fe9type:discriminator mandatory on every controllers entry7ab10feeFinal YAML shape per entry:
Fixes after code review
A second pass after the initial review surfaced eleven follow-up issues (#361–#371). Each was triaged in parallel and fixed on its own branch; the commits below land them on
multiple-controllers. #362 was decision-only and resolved as already-implemented by commit439f2fe9above (option 2:type:discriminator mandatory in all modes).c8be8d69142e4e8dd341fff82cf219bcf055141004d36b2cc87711ef6907741db284afe54853d0eb6db26ccfHow to review
Recommend commit-by-commit — each commit's body explains its sub-module and the tests that cover it, and the suite is green at every commit.
(GK: yeah - or maybe just go through the tutorial!)
Suggested follow-up: real-world validation via fastcs-catio
A good shake-down for this branch is to bump
fastcs-catioto a build of this version and run it against real hardware:controllers:YAML, multi-classlaunch(),controller_apisplural accessor,Controller.id-seeded paths) reads sensibly in a non-trivial app.Feed the resulting diff and any rough edges back into a new docs page "How to migrate to fastcs 0.11.0" — at minimum covering the YAML schema change (
controller:→controllers:keyed by id,type:discriminator), theController.id/ path semantics, the unifiedTransport.connect(list[ControllerAPI])signature, and the EPICS CA prefix derivation (pv_prefix_from_path).🤖 Generated with Claude Code